Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DPDK: install path fixes for meson and Ubuntu 24.04 #3598

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

DPDK: install path fixes for meson and Ubuntu 24.04 #3598

wants to merge 16 commits into from
+104 −12

Conversation

mcgov
Copy link
Collaborator

@mcgov mcgov commented Jan 13, 2025 •
edited
Loading

Fixing some bugs which were noted after testing on Ubuntu 24.04:

  • 'meson' is occasionally packaged as 'meson' but it is being standardized as 'python3-meson' in future releases. This PR fixes the installation path for Meson to account for an prefer the packaged version if it's available and the correct version.
  • linux-modules-extra-azure requires a restart, dependencies occasionally do. So add a 'needs_restart' parameter to the dependency installer to allow rebooting before continuing to install dependencies.

squirrelsc
squirrelsc reviewed Jan 15, 2025
View reviewed changes
lisa/tools/meson.py Outdated Show resolved Hide resolved
@mcgov mcgov marked this pull request as ready for review January 21, 2025 18:52
@mcgov mcgov requested a review from LiliDeng as a code owner January 21, 2025 18:52
@LiliDeng
Copy link
Collaborator

LiliDeng commented Jan 23, 2025 •
edited
Loading

I still saw issues when run test against canonical ubuntu-24_04-lts server 24.04.202411030

2025-01-23T03:45:57.2367877Z 2025-01-23 03:45:57.233[5488][DEBUG] lisa.env[generated_0].node[0].cmd[2463] cmd: ['pip3', 'install', '-q', 'pyelftools'], cwd: None, shell: False, sudo: False, nohup: False, posix: True, remote: True, encoding: utf-8
2025-01-23T03:45:57.5747063Z 2025-01-23 03:45:57.573[5492][DEBUG] lisa.env[generated_0].node[0].cmd[2463].stdout error: externally-managed-environment
2025-01-23T03:45:57.5752932Z 2025-01-23 03:45:57.573[5492][DEBUG] lisa.env[generated_0].node[0].cmd[2463].stdout � This environment is externally managed
2025-01-23T03:45:57.5757649Z 2025-01-23 03:45:57.573[5492][DEBUG] lisa.env[generated_0].node[0].cmd[2463].stdout \u2570\u2500> To install Python packages system-wide, try apt install
2025-01-23T03:45:57.5762505Z 2025-01-23 03:45:57.573[5492][DEBUG] lisa.env[generated_0].node[0].cmd[2463].stdout     python3-xyz, where xyz is the package you are trying to
2025-01-23T03:45:57.5765328Z 2025-01-23 03:45:57.573[5492][DEBUG] lisa.env[generated_0].node[0].cmd[2463].stdout     install.
2025-01-23T03:45:57.5771737Z 2025-01-23 03:45:57.573[5492][DEBUG] lisa.env[generated_0].node[0].cmd[2463].stdout     If you wish to install a non-Debian-packaged Python package,
2025-01-23T03:45:57.5778365Z 2025-01-23 03:45:57.573[5492][DEBUG] lisa.env[generated_0].node[0].cmd[2463].stdout     create a virtual environment using python3 -m venv path/to/venv.
2025-01-23T03:45:57.5783775Z 2025-01-23 03:45:57.573[5492][DEBUG] lisa.env[generated_0].node[0].cmd[2463].stdout     Then use path/to/venv/bin/python and path/to/venv/bin/pip. Make
2025-01-23T03:45:57.5788002Z 2025-01-23 03:45:57.573[5492][DEBUG] lisa.env[generated_0].node[0].cmd[2463].stdout     sure you have python3-full installed.
2025-01-23T03:45:57.5794100Z 2025-01-23 03:45:57.573[5492][DEBUG] lisa.env[generated_0].node[0].cmd[2463].stdout     If you wish to install a non-Debian packaged Python application,
2025-01-23T03:45:57.5799329Z 2025-01-23 03:45:57.573[5492][DEBUG] lisa.env[generated_0].node[0].cmd[2463].stdout     it may be easiest to use pipx install xyz, which will manage a
2025-01-23T03:45:57.5804530Z 2025-01-23 03:45:57.573[5492][DEBUG] lisa.env[generated_0].node[0].cmd[2463].stdout     virtual environment for you. Make sure you have pipx installed.
2025-01-23T03:45:57.5810086Z 2025-01-23 03:45:57.573[5492][DEBUG] lisa.env[generated_0].node[0].cmd[2463].stdout     See /usr/share/doc/python3.12/README.venv for more information.
2025-01-23T03:45:57.5822285Z 2025-01-23 03:45:57.573[5492][DEBUG] lisa.env[generated_0].node[0].cmd[2463].stdout note: If you believe this is a mistake, please contact your Python installation or OS distribution provider. You can override this, at the risk of breaking your Python installation or OS, by passing --break-system-packages.
2025-01-23T03:45:57.5825567Z 2025-01-23 03:45:57.573[5492][DEBUG] lisa.env[generated_0].node[0].cmd[2463].stdout hint: See PEP 668 for the detailed specification.
2025-01-23T03:45:57.6262525Z 2025-01-23 03:45:57.625[5488][DEBUG] lisa.env[generated_0].node[0].cmd[2463] execution time: 0.389 sec, exit code: 1

squirrelsc
squirrelsc reviewed Jan 24, 2025
View reviewed changes
lisa/operating_system.py Outdated
install_result = self._node.execute(
command, shell=True, sudo=True, timeout=timeout
)
assert_that(install_result.exit_code).described_as(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

install_result.assert_exist_code implemented the similar error message, please reuse it.

squirrelsc
squirrelsc reviewed Jan 24, 2025
View reviewed changes
lisa/operating_system.py Outdated
remove_packages = " ".join(packages)
command += f" rm {remove_packages}"
self.wait_running_process("zypper")
install_result = self._node.execute(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

install_result -> uninstall_result

squirrelsc
squirrelsc reviewed Jan 24, 2025
View reviewed changes
lisa/tools/meson.py Outdated
@@ -15,13 +15,18 @@


class Meson(Tool):
_minimum_version = VersionInfo(major=0, minor=52, patch=0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use lisa.util.parse_version.

squirrelsc
squirrelsc reviewed Jan 24, 2025
View reviewed changes
lisa/tools/meson.py
if posix_os.package_exists(pkg, minimum_version=self._minimum_version):
package_installed = pkg
break
elif posix_os.is_package_in_repo(pkg):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

elif to if is the same, but it means the two if blocks are actually independent.

mcgov added 4 commits January 27, 2025 07:45
@mcgov
make package_exists version check simpler
3371aca
@mcgov
DPDK: prefer newer pyelftools package
1b4c29b
@mcgov
Use helper function instead of instantiating the class directly
2979507
@mcgov
check_exists( ..., minimum_version) considered harmful
097130f 
Undoing a change I added which enabled a _minimum_version check on
_check_exists. This is a bad system because you'll end up needing to do
things like this:

if not posix_os._check_exists(pkg, minimum_version=...):
	# Now what? Was it installed or not?
	# Was it installed and just the wrong version?

The ultimate fix should be is_package_in_repo(pkg, minimum_version)
Since we should be checking if an available package is the right version
_before_ installing it blindly.

This all seems out-of-scope for fixing a bug installing meson
in _just_ the DPDK suite. So, I'm removing it. We can move the fixed
install pattern to the operating_system.py level later if anyone else
needs it.
@mcgov
Copy link
Collaborator Author

mcgov commented Jan 29, 2025

I still saw issues when run test against canonical ubuntu-24_04-lts server 24.04.202411030
@LiliDeng Correct! Fixed this with the latest two commits. Apologies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@squirrelsc squirrelsc squirrelsc left review comments

@LiliDeng LiliDeng Awaiting requested review from LiliDeng LiliDeng is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mcgov @LiliDeng @squirrelsc